Fix slots issue with deferred base#20573
Conversation
Fixes python#17121 Generated by codex
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
| slots_defined_by_plugin = existing_slots is not None and existing_slots.plugin_generated | ||
| if existing_slots is not None and not slots_defined_by_plugin: |
There was a problem hiding this comment.
I don't really get how the original was wrong. Like why is info.slots a thing (or __slots__ in info.names?)... And why do we have to care about it being plugin generated, is this plugin being run multiple times? (I see the old code had info.slots != generated_slots for this too. That seems odd)
There was a problem hiding this comment.
Yes, I think the deferral led to this being called multiple times
There was a problem hiding this comment.
Oh I see, if this is run multiple times then __slots__ will be added then trigger this if statement (in the old code). Is there a reason we can't store "we've processed this dataclass already" and avoid rerunning this code, if that's indeed the case? That might stop anything similar from happening.
There was a problem hiding this comment.
Seems worth looking into! Codex generated this commit and it seemed like a more correct version of the previously existing code that attempted to make this logic idempotent, so I opened the PR
There was a problem hiding this comment.
This is documented in DataclassTransformer fwiw
There was a problem hiding this comment.
Ok, I see the other things have things like:
and (
"__match_args__" not in info.names or info.names["__match_args__"].plugin_generated
)
(which is basically what happens here)
Fixes python#17121 Generated by codex + hand simplification commit
Fixes python#17121 Generated by codex + hand simplification commit
3 participants
Fixes #17121
Generated by codex + hand simplification commit